Added remove skin button and ability to delete skin from skin directory#306
Open
henbotb wants to merge 1 commit intoMoulberry:masterfrom
Open
Added remove skin button and ability to delete skin from skin directory#306henbotb wants to merge 1 commit intoMoulberry:masterfrom
henbotb wants to merge 1 commit intoMoulberry:masterfrom
Conversation
Moulberry
requested changes
Mar 18, 2026
| } | ||
|
|
||
| fn get_path_from_skin(backend: &BackendState, skin: Arc<[u8]>) -> Option<PathBuf> { | ||
| let Ok(read_dir) = std::fs::read_dir(&backend.directories.skin_library_dir) else { |
Owner
There was a problem hiding this comment.
Scanning the whole directory like this seems not ideal, perhaps the skin manager should maintain a map from skin -> path
Author
There was a problem hiding this comment.
Totally agree, and had the same thought while programming this. I ended up opting for this solution as the it's a fairly low-access function and I didn't see an obvious way to implement something like a map without having to add an extra parameter to a bunch of closures. That said, I will look into something less costly.
If you have any ideas for the structure, let me know.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a trash can icon on the bottom left of every skin option in the skin menu to delete the skin.
With this implementation, however, the click event on the skin png is also registered, meaning that it registers it as the selected skin whilst removing it, which technically doesn't matter but would be nice to fix for QOL purposes (and also minor performance given the skin widget wouldn't need to reload). I read a few ways to suspend the click event / stop click propagation, but didn't find much documentation on them.
This would also help resolve another minor visual bug where if you delete a skin when it's in a selected state, it removes the selection as a whole.